This is perfectly expected to set a thing status in thing handler initialize() before returning. So the code you removed is correct.
The problem is a bad bridge/thing implementation: your thing handler is apparently not checking the bridge status.
I can try to propose an alternative and proper correction of your problem if you are available to test it.

There is a large PR #11963 with new functionality for the Niko Home Control binding that has been open, waiting for review, for a while.

Prefer several small PRs and you will get a faster feedback.
This is what you did with this PR and we should conclude soon.

The problem is a bad bridge/thing implementation: your thing handler is apparently not checking the bridge status.

I was wrong, it is checked ... but very late in the thing initialization. Maybe the problem.
You are updating very often the thing status in this binding, probably more than expected. Could be another source of problems.

The problem is a bad bridge/thing implementation: your thing handler is apparently not checking the bridge status.

I was wrong, it is checked ... but very late in the thing initialization. To be investigated.
You are updating very often the thing status in this binding, probably more than expected. Could be another source of problems.

Setting the thing status to UNKNOWN early in bridge initialization causes the thing initialization to start. And that is faster than the bridge initialization, therefore the things end offline. Removing it corrects it all.

@lolodomo - I added a few comments which are now resolved. Please have a final look as you also had some concerns - and merge if everything is okay with you.

@lolodomo Is there anything left you would want me to look into? I looked again at the sequence of setting the bridge and thing status. There is nothing obvious to simplify it when I look at it. Let me know if you have suggestions.

The PR title is already a derivation to the recommendation ;) Yes, a valid thing status must be set quickly. If the full initializatoon of the thing handler requires time the thing status can then be updated again later by code run in a separate thread.

The initialize methods do return quickly. And they ultimately do set the thing status, but in a separate thread to allow the initialize method to return quickly. Is there an obligation to already set the thing status in the same thread the initialize method was called from? If that’s the case, I misinterpreted and I will have to do many more changes. I thought returning quickly just had to do with not keeping a thread occupied that would initialize other things in other bindings.

@lolodomo Setting the ting status to unknown was introduced in the binding 7 months ago. It wasn’t there before. The binding pre-dates the creation of the archetype. I read the reasoning for setting thing status unknown (unit tests), but I don’t agree it is right to set it in this case. Introducing it introduced a bug in the binding, which I try to revert here.
I could keep it in the thing handlers, but not int bridge handler. That is where the real problem is, because it caused the things to initialize too early.

Is there an obligation to already set the thing status in the same thread the initialize method was called from? If that’s the case, I misinterpreted and I will have to do many more changes. I thought returning quickly just had to do with not keeping a thread occupied that would initialize other things in other bindings.

This is at least the clear recommendation for binding developers, here is the code from the binding template:

    public void initialize() {
        config = getConfigAs(${bindingIdCamelCase}Configuration.class);

        // TODO: Initialize the handler.
        // The framework requires you to return from this method quickly, i.e. any network access must be done in
        // the background initialization below.
        // Also, before leaving this method a thing status from one of ONLINE, OFFLINE or UNKNOWN must be set. This
        // might already be the real thing status in case you can decide it directly.
        // In case you can not decide the thing status directly (e.g. for long running connection handshake using WAN
        // access or similar) you should set status UNKNOWN here and then decide the real status asynchronously in the
        // background.

        // set the thing status to UNKNOWN temporarily and let the background task decide for the real status.
        // the framework is then able to reuse the resources from the thing handler initialization.
        // we set this upfront to reliably check status updates in unit tests.
        updateStatus(ThingStatus.UNKNOWN);

        // Example for background initialization:
        scheduler.execute(() -> {
            boolean thingReachable = true; // <background task with long running initialization here>
            // when done do:
            if (thingReachable) {
                updateStatus(ThingStatus.ONLINE);
            } else {
                updateStatus(ThingStatus.OFFLINE);
            }
        });

https://github.com/openhab/openhab-core/blob/main/tools/archetype/binding/src/main/resources/archetype-resources/src/main/java/internal/__bindingIdCamelCase__Handler.java#L80

The comment in the template looks to me really clear ;)

before leaving this method a thing status from one of ONLINE, OFFLINE or UNKNOWN must be set

@lolodomo Then the developer documentation should be changed: https://www.openhab.org/docs/developer/bindings/
Again, please, I am willing to keep status UNKNOWN for the thing handlers, but it leads to wrong behavior for the bridge handler (calling the thing handlers initialize too early). Do you also want me to keep it in the bridge handler? I read the argument in the comment in the archetype implementation again. This was introduced to avoid issues with unit tests, that is the only reason. It cannot be right to break bindings with no unit tests because of this.
If this is a hard rule, it means all long running bridge initializations will force depending thing initializations to have extra logic to wait for the bridge to be fully initialized. This essentially voids the advantage of the thing-bridge dependency.

I read the argument in the comment in the archetype implementation again. This was introduced to avoid issues with unit tests, that is the only reason.

Why do you say it is the only reason ? I don't think so.

As reviewer, I think my main role is to focus on the good practices and guidelines are followed. So I will not approve this PR myself.

Now, this is only my feedback and of course another reviewer can approve and merge this PR. @jlaur was already involved in the review.

f this is a hard rule, it means all long running bridge initializations will force depending thing initializations to have extra logic to wait for the bridge to be fully initialized. This essentially voids the advantage of the thing-bridge dependency.

There is no particular difficulty when the bridge/thing initialization is well implemented. This case is very common and properly handled in many bindings. In your thing handler, you just have to first check the status of the bridge. If not yet ONLINE, you set your thing status to OFFLINE/BRIDGE_OFFLINE.

I could keep it in the thing handlers, but not int bridge handler. That is where the real problem is, because it caused the things to initialize too early.

Yes, of course, but in this case, you set your thing status to OFFLINE/BRIDGE_OFFLINE.
When your bridge status is changed, this will trigger your thing handler and you can run your thing initialization stuff.

@lolodomo I don't want to argue this any further. I developed this binding something like 5 years ago, based on the documentation at that point in time (which still is the same in the documentation). It had always worked well and reliably. On Oct. 9th 2021, in another PR #11319, you asked me to put the thing status UNKNOWN lines in. From that point onwards, the binding stopped starting reliably. I now for the first time see the archetype, which was only created after my binding got created, reviewed and accepted, and I don't agree with that specific line in it.
I agree things can be made to work, but more of the effort is on the binding than on the framework in that case. To make it work reliably again, I need to lift much of the initialization logic from the thing handler initialize methods and only call it from initialize when the bridge is out of status UNKOWN, or from bridgeStatusChanged. I think it makes things more complicated then it needs to be, but will do so.

I agree things can be made to work, but more of the effort is on the binding than on the framework in that case. To make it work reliably again, I need to lift much of the initialization logic from the thing handler initialize methods and only call it from initialize when the bridge is out of status UNKOWN, or from bridgeStatusChanged. I think it makes things more complicated then it needs to be, but will do so.

Wait for @jlaur feedback, he is maybe ok with your changes and will then merge your PR.

Wait for @jlaur feedback, he is maybe ok with your changes and will then merge your PR.

I will be away today, but will be back in the evening. If there will still be any doubts then, I will catch up and try to provide some feedback.

For reference, here is the original discussion on why the requirement was introduced in the archetype: PR #6160

@lolodomo @jlaur I think I have made the necessary modifications to be able to set the thing status early now. As far as I could test, it still behaves as I would want it to behave. Could you check it out?

I did not yet check the final PR but I assume the PR title has to be updated (it will appear in the changelog for the user) ?